Fixes search usage telemetry #1427

Flyingliuhub · 2022-04-05T18:57:17Z

Signed-off-by: Tao liu liutaoaz@amazon.com

Description

Search Telemetry is used to analyze performance for sucess/failed search request in Dashboards. It will save telemetry data into .kibana_1 index, there are thousands of concurrent search request from Dashboards. It causes significant load in OpenSearch cluster. We have been seen some case 1/3 Search/Update request of total are come from this. This PR will suppress the search usage telemetry based on the switch defined on opensearch_dashboard.yml or data plugin config.

Issues Resolved

resolves #928

Check List

yarn test:jest

Test Suites: 1 skipped, 1432 passed, 1432 of 1433 total
Tests:       24 skipped, 9 todo, 10721 passed, 10754 total
Snapshots:   2452 passed, 2452 total
Time:        241.591 s
Ran all test suites.
Done in 243.82s.

yarn test:jest_integration

Test Suites: 50 passed, 50 total
Tests:       5 skipped, 438 passed, 443 total
Snapshots:   77 passed, 77 total
Time:        294.497 s
Ran all test suites.
         │ info [opensearch] cleanup complete
Done in 296.78s.

opensearch_dashboard.yml enabled value	data plugin config enabled value	Results
TRUE	FALSE	Passed
FALSE	FALSE	Passed
TRUE	TRUE	Passed
FALSE	TRUE	Passed
NONE	TRUE	Passed
NONE	FALSE	Passed

Signed-off-by: Tao liu <liutaoaz@amazon.com>

seraphjiang · 2022-04-05T20:01:14Z

I like the this table, explain how to use this very well. what's our recommendation to make it a user manual or doc
@kavilla @seanneumann

opensearch_dashboard.yml enabled value	data plugin config enabled value	Results
TRUE	FALSE	Passed
FALSE	FALSE	Passed
TRUE	TRUE	Passed
FALSE	TRUE	Passed
NONE	TRUE	Passed
NONE	FALSE	Passed

mihirsoni · 2022-04-05T21:09:39Z

@Flyingliuhub Thanks this is super nice, it would be great to add some UTs to ensure the config works in expected behavior.

config/opensearch_dashboards.yml

src/plugins/data/config.ts

src/plugins/data/server/search/collectors/usage.ts

kavilla · 2022-04-05T22:48:10Z

@Flyingliuhub Thanks this is super nice, it would be great to add some UTs to ensure the config works in expected behavior.

+1

kavilla · 2022-04-05T22:52:35Z

Do we need add this config here for Docker containers:

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/bin/opensearch-dashboards-docker#L17

and then update

https://github.com/opensearch-project/opensearch-build/blob/main/docker/release/config/opensearch-dashboards/opensearch-dashboards-docker-entrypoint.sh#L21 ?

Flyingliuhub · 2022-04-06T16:43:50Z

Do we need add this config here for Docker containers:

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/bin/opensearch-dashboards-docker#L17

and then update

https://github.com/opensearch-project/opensearch-build/blob/main/docker/release/config/opensearch-dashboards/opensearch-dashboards-docker-entrypoint.sh#L21 ?

We used the default value from config, do we need to change docker file ?

Signed-off-by: Tao liu <liutaoaz@amazon.com>

Flyingliuhub · 2022-04-06T17:01:48Z

Thanks, I added some unit tests.

Signed-off-by: Tao liu <liutaoaz@amazon.com>

seraphjiang

LGTM

src/plugins/data/server/search/collectors/usage.ts

mihirsoni

LGTM !! Thanks for UT.

Flyingliuhub · 2022-04-07T03:26:31Z

Thanks Mihir.

kavilla · 2022-04-07T09:22:10Z

Is this still valid: https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/data/server/server.api.md?plain=1#L1104

I know we don't currently use these docs but I think @ashwin-pc just made a proposal here: opensearch-project/documentation-website#492 to start including this docs which I agree with. So might be worth to update this doc here.

kavilla · 2022-04-07T09:23:35Z

I like the this table, explain how to use this very well. what's our recommendation to make it a user manual or doc @kavilla @seanneumann
opensearch_dashboard.yml enabled value data plugin config enabled value Results
TRUE FALSE Passed
FALSE FALSE Passed
TRUE TRUE Passed
FALSE TRUE Passed
NONE TRUE Passed
NONE FALSE Passed

You mean like how to let the public know? Yeah right now the current strategy has been create an issue with the technical details in this repo: https://github.com/opensearch-project/documentation-website/issues/new?assignees=&labels=enhancement%2C+untriaged&template=FEATURE_REQUEST_TEMPLATE.md&title=%5BFEATURE%5D

src/plugins/data/config.ts

src/plugins/data/server/search/collectors/usage.ts

kavilla

I have 1 nit about the default value and 1 comment I think you should address about the potentially undefined case but I'm still ecstatic that this is in here!

So don't want to block this. I think if you just make a commit on top of this should not wipe out my approval. Just don't rebase, that will definitely clear everyone's approval.

Signed-off-by: Tao liu <liutaoaz@amazon.com>

src/plugins/data/server/search/collectors/usage.ts

fix search usage telemetry

4ead66f

Signed-off-by: Tao liu <liutaoaz@amazon.com>

Flyingliuhub requested a review from a team as a code owner April 5, 2022 18:57

Flyingliuhub changed the title ~~fix search usage telemetry~~ Fixes search usage telemetry Apr 5, 2022

Flyingliuhub requested a review from kavilla April 5, 2022 18:58

seraphjiang requested review from ananzh, sichend and zengyan-amazon April 5, 2022 19:54

tianleh reviewed Apr 5, 2022

View reviewed changes

config/opensearch_dashboards.yml Outdated Show resolved Hide resolved

kavilla reviewed Apr 5, 2022

View reviewed changes

src/plugins/data/config.ts Outdated Show resolved Hide resolved

src/plugins/data/server/search/collectors/usage.ts Show resolved Hide resolved

seraphjiang added the v2.0.0 label Apr 6, 2022

seraphjiang assigned Flyingliuhub Apr 6, 2022

Flyingliuhub closed this Apr 6, 2022

Flyingliuhub reopened this Apr 6, 2022

add unit tests and fixes comments

66873a7

Signed-off-by: Tao liu <liutaoaz@amazon.com>

add newline at the end of usage.test.ts

d22f483

Signed-off-by: Tao liu <liutaoaz@amazon.com>

Flyingliuhub requested a review from kavilla April 6, 2022 20:17

add docker var change

86196c8

Signed-off-by: Tao liu <liutaoaz@amazon.com>

seraphjiang previously approved these changes Apr 6, 2022

View reviewed changes

tianleh previously approved these changes Apr 7, 2022

View reviewed changes

mihirsoni reviewed Apr 7, 2022

View reviewed changes

src/plugins/data/server/search/collectors/usage.ts Show resolved Hide resolved

mihirsoni previously approved these changes Apr 7, 2022

View reviewed changes

kavilla reviewed Apr 7, 2022

View reviewed changes

src/plugins/data/config.ts Show resolved Hide resolved

kavilla reviewed Apr 7, 2022

View reviewed changes

src/plugins/data/config.ts Show resolved Hide resolved

kavilla reviewed Apr 7, 2022

View reviewed changes

src/plugins/data/server/search/collectors/usage.ts Outdated Show resolved Hide resolved

kavilla reviewed Apr 7, 2022

View reviewed changes

src/plugins/data/server/search/collectors/usage.ts Outdated Show resolved Hide resolved

kavilla previously approved these changes Apr 7, 2022

View reviewed changes

update check configuration

96b3fd4

Signed-off-by: Tao liu <liutaoaz@amazon.com>

Flyingliuhub dismissed stale reviews from kavilla, mihirsoni, tianleh, and seraphjiang via 96b3fd4 April 7, 2022 15:57

kavilla approved these changes Apr 7, 2022

View reviewed changes

seraphjiang approved these changes Apr 7, 2022

View reviewed changes

Flyingliuhub merged commit f0ea74e into opensearch-project:main Apr 7, 2022

Flyingliuhub mentioned this pull request Apr 7, 2022

add new data.search.usageTelemetry.enabled to opensearch_dashboards_vars opensearch-project/opensearch-build#1931

Merged

1 task

zengyan-amazon reviewed Apr 7, 2022

View reviewed changes

src/plugins/data/server/search/collectors/usage.ts Show resolved Hide resolved

Flyingliuhub mentioned this pull request Apr 7, 2022

[FEATURE] Document new configuration value “data.search.usageTelemetry.enabled” in the opensearch_dashboard.yml opensearch-project/documentation-website#494

Closed

This was referenced Feb 8, 2023

[search telemetry] Fixes search telemetry's observable object that won't be GC-ed #3390

Merged

Add memory leak detection to unit tests #3405

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes search usage telemetry #1427

Fixes search usage telemetry #1427

Flyingliuhub commented Apr 5, 2022 •

edited

Loading

seraphjiang commented Apr 5, 2022

mihirsoni commented Apr 5, 2022

kavilla commented Apr 5, 2022

kavilla commented Apr 5, 2022

Flyingliuhub commented Apr 6, 2022

Flyingliuhub commented Apr 6, 2022

seraphjiang left a comment

mihirsoni left a comment •

edited

Loading

Flyingliuhub commented Apr 7, 2022

kavilla commented Apr 7, 2022

kavilla commented Apr 7, 2022

kavilla left a comment

Fixes search usage telemetry #1427

Fixes search usage telemetry #1427

Conversation

Flyingliuhub commented Apr 5, 2022 • edited Loading

Description

Issues Resolved

Check List

seraphjiang commented Apr 5, 2022

mihirsoni commented Apr 5, 2022

kavilla commented Apr 5, 2022

kavilla commented Apr 5, 2022

Flyingliuhub commented Apr 6, 2022

Flyingliuhub commented Apr 6, 2022

seraphjiang left a comment

Choose a reason for hiding this comment

mihirsoni left a comment • edited Loading

Choose a reason for hiding this comment

Flyingliuhub commented Apr 7, 2022

kavilla commented Apr 7, 2022

kavilla commented Apr 7, 2022

kavilla left a comment

Choose a reason for hiding this comment

Flyingliuhub commented Apr 5, 2022 •

edited

Loading

mihirsoni left a comment •

edited

Loading